-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
arbitrum nitro support: implement the jsonrpc client logic to get required data #67
Conversation
ccbd98b
to
d913247
Compare
5bc46b4
to
6ea9768
Compare
…required jsonrpc messages Arbitrum Nitro doesn't have eth_getBlockReceipts, so we need to make a variable number of jsonRPC requests, we call eth_getTransactionReceipt for each transaction in the block. For this we parse the eth_getBlockByNumber response and extract the tx hashes
6ea9768
to
b6f85bf
Compare
We don't have much in the way of tests for the code in the clients at the moment. I'd be keen for us to have something covering that as we have more stack specific code accruing there. |
|
||
group.Go(func() error { | ||
errCh := make(chan error, 1) | ||
c.wrkPool.Submit(func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love us to have something like RxGo (but maintained) or go-streams or similar to make some of this channel + errgroup + ant stuff simpler. Even if we wrote a small utility function that covered the basic pattern we keep using.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
return models.RPCBlock{}, err | ||
} | ||
|
||
txHashes, err := getTransactionHashes(results[0].Bytes()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we start working on getting the transactions once we complete all methods from above (eth_getBlockByNumber
, debug_traceBlockByNumber
). But in reality we only need eth_getBlockByNumber
. Does it make any difference waiting all methods to complete, compared to start getting transactions once we get the eth_getBlockByNumber
response ?
@@ -56,8 +56,7 @@ func (c *OpStackClient) BlockByNumber(ctx context.Context, blockNumber int64) (m | |||
results := make([]*bytes.Buffer, len(methods)) | |||
for i, method := range methods { | |||
results[i] = c.bufPool.Get().(*bytes.Buffer) | |||
defer c.bufPool.Put(results[i]) | |||
results[i].Reset() | |||
defer c.putBuffer(results[i]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this makes any difference, but here you also defering the buf.Reset()
compared to previous implementation. Just making sure this is intentional.
I'm merging this, I'm doing more PRs ontop that add extra tests and will also refactor this part of the code for simplicity |
Arbitrum Nitro doesn't have eth_getBlockReceipts, so we need to make a variable number of jsonRPC requests, we call eth_getTransactionReceipt for each transaction in the block.
For this we parse the eth_getBlockByNumber response and extract the tx hashes
This PR is ontop of:
#65
And it also requires: #64, because each block message can now be of variable size.